Repository navigation
feat(libc): wire the codecvt<wchar_t,char> facet vtable - #11172
Open
00xDisaster wants to merge 5 commits into
Open
00xDisaster wants to merge 5 commits into
00xDisaster wants to merge 5 commits into
Conversation
Progress report (0278ca9 → d7562dc)📈 System libraries: 26.19% (+0.02%, +7 implemented functions) ✅ 7 implemented
⚪ 4 behaviourally empty (constant return or throw)
Changed libraries (implemented exports)
|
00xDisaster
force-pushed
the
feat/libc-codecvt-wide-vtable
branch
from
October 25, 2026 20:40
e0d580e to
6ad55bf
Compare
MarcZX10
reviewed
Oct 25, 2026
MarcZX10
left a comment
MarcZX10
left a comment
Contributor
There was a problem hiding this comment.
I reviewed head 6ad55bff263cb8a47ce17a2c9391b9eacc4bfbba, including the wide facet slot layout, UTF-8 conversion loops, partial-input continuation, output buffer handling, and the 16-byte guest conversion-state storage. The new tests exercise the vtable and the relevant one-, two-, and three-byte cases. I did not find a concrete correctness issue in the reviewed changes.
Independent checks here: C++20 syntax compilation of LocaleSupport.cpp and GuestLocale.cpp, plus git diff --check upstream/main...HEAD, passed. The author reports the six focused locale suites passed. Hosted conventions, labels, and report checks now pass; I do not see hosted product-build results on this head. I’m leaving a comment-only review pending runtime/build evidence rather than approving without it.
Contributor
Author
|
Local build evidence at
Hosted relinker (Ubuntu/Windows), relinker-asan and Linux register-context checks pass on this head; the macOS jobs are still queued. The |
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
Contributor
|
Follow-up to my review: the outstanding macOS Intel Debug register-context check has now passed. Linux/macOS/Windows relinker, relinker-ASan, Ubuntu Debug/Release and macOS Intel Debug/Release register-context, and both macOS write-watch jobs pass. Full Linux and Windows product jobs are skipped by the path policy; they are not build results for this change. |
12 tasks done
Contributor
|
Windows run of head
AI-assisted: yes (Claude Code); results above were run on this machine. |
Contributor
Author
|
@MarcZX10 the build and runtime evidence you asked for is now available on
All hosted relinker, ASan, register-context and write-watch checks pass; the full Linux/Windows product jobs are skipped by the PR path policy. The change still declares its textual dependency on open #10165 and does not modify that PR. Please let me know if this covers the runtime/build evidence request. |
Contributor
Author
|
All hosted checks are green on |
Contributor
|
Windows result for head Host: Windows 11 26300, Intel i7-12700K, WinLibs GCC 15.2.0 (x86_64-ucrt-posix-seh, r6), Release, The tree also carried the open PRs #10780, #10729, #11614, #10631, #10582, #11547, #11584, #11572, all merged cleanly, so the run is a joint one; the tests of this PR are AI-assisted: yes (Claude Code), the runs and the numbers above are from this machine. |
This was referenced Oct 27, 2026
Open
Contributor
Author
|
@MarcZX10 @maskym I added the independent Windows result to Tested: |
12 tasks done
MarcZX10
suggested changes
Oct 29, 2026
MarcZX10
left a comment
MarcZX10
left a comment
Contributor
There was a problem hiding this comment.
Re-checked — new head, changes still requested
The independent Windows guest_locale result is helpful, but both the PR and that result use base 74ac0e7912, while current main changed the locale implementation, tests, and debt file.
What I ran
- Reviewed exact PR head
6ad55bff26against currentmainedf6804cf52c62eacafc592dd6142cb4efe5bd05. - Ran
git merge-base,git merge-tree --write-tree,git diff --name-statusfor shared paths, andgit diff --check. - Checked current hosted workflow results. I did not execute locale tests locally.
Results
| Check | Result |
|---|---|
| PR base / current main | 74ac0e7912 / edf6804cf5 |
| Merge preview | Clean; tree 7dbdf8329a4897de163318daeca6595e2599c8e9 |
| Current-main changes on PR paths | LocaleSupport.cpp, GuestLocale.cpp, and docs/dev/debt/libc.md |
| Diff check | Passed; no output |
| Hosted conventions, labels, eligibility, report, relinker, ASan, register-context, write-watch | Passed |
| Hosted product Windows/Linux jobs | Skipped |
| Author-reported Windows result | guest_locale passes as part of the 160-test locale/library suite on the PR head, but its integration base predates current main |
Unedited available command output
$ git merge-base upstream/main refs/review/upstream-11172
74ac0e7912a5eaa453fff872f4d4fe803154b08c
$ git merge-tree --write-tree upstream/main refs/review/upstream-11172
7dbdf8329a4897de163318daeca6595e2599c8e9
$ git diff --name-status 74ac0e7912..upstream/main -- core/libs/prx/libc/src/LocaleSupport.cpp core/libs/tests/GuestLocale.cpp docs/dev/debt/libc.md
M core/libs/prx/libc/src/LocaleSupport.cpp
M core/libs/tests/GuestLocale.cpp
M docs/dev/debt/libc.md
$ git diff --check upstream/main...refs/review/upstream-11172
$ gh pr checks 11172 -R boykopovar/AnyPS5
conventions pass 38s
eligibility pass 4s
labels pass 9s
register-context (macos-15-intel, Debug) pass 1m18s
register-context (macos-15-intel, Release) pass 1m5s
register-context (ubuntu-latest, Debug) pass 28s
register-context (ubuntu-latest, Release) pass 25s
relinker (macos-latest, clang++) pass 3m40s
relinker (macos-latest, clang++, x86_64) pass 2m34s
relinker (ubuntu-latest, g++) pass 3m39s
relinker (windows-latest, g++) pass 4m32s
relinker-asan pass 2m23s
write-watch (macos-15) pass 36s
linux skipping 0
windows skipping 0
report pass 31s
write-watch (macos-15-intel) pass 1m4s
Negative control
| Check | Result |
|---|---|
| New test against current-main base | Not run; the available author result is for the earlier base |
Findings
Blocking
- Please rebase on current
mainand rerunguest_localeon the exact refreshed base and head. Main changed the locale implementation and the shared regression test since74ac0e7912, so the earlier result does not verify the current integration.
Non-blocking
- None found in the clean merge preview.
Sources
- Author’s Windows test update
- Current-main locale support
- Current-main locale regression test
- PR-head locale change
Not verified
- The exact-head locale result after integrating current
main.
To get this to a good state
- Refresh the branch, run and report
guest_localeon current base and change, and update the exact commit hashes in the description.
00xDisaster
force-pushed
the
feat/libc-codecvt-wide-vtable
branch
from
October 29, 2026 11:22
6ad55bf to
fdc62fb
Compare
Contributor
Author
|
@MarcZX10 refreshed onto current |
XairRrR
approved these changes
Oct 29, 2026
XairRrR
left a comment
XairRrR
left a comment
Contributor
There was a problem hiding this comment.
Reviewed — approved
The refreshed head merges cleanly into current main, builds, passes the six locale suites, and the new guest_locale checks fail against main's LocaleSupport.cpp. I found nothing blocking.
What I ran
Head fdc62fb1048cd66e6969b1e4819f53d979ec1ca3 merged with --no-ff into main ba5b5330b37724bca4b99f30faab504653635a04 in a clean worktree. Release, GCC 16.2.1, Ninja, BUILD_TESTING=ON. Arch Linux 7.2.11-zen, Intel Core i9-14900KF.
git merge --no-ff --no-edit refs/review/11172
git diff --check origin/main...refs/review/11172
python3 tools/check_conventions.py --base origin/main
cmake --build build -j10 --target guest_locale_tests guest_multibyte_locale_tests guest_wide_conversion_tests guest_wide_formatting_tests guest_wide_string_tests guest_wide_stream_scan_tests libc
cmake --build build -j10 --target libs
ctest --test-dir build -j4 -R '^(guest_locale|guest_multibyte_locale|guest_wide_formatting|guest_wide_strings|guest_wide_conversion|guest_wide_stream_scan)$' --output-on-failure
# negative control
git checkout origin/main -- core/libs/prx/libc/src/LocaleSupport.cpp
cmake --build build -j10 --target guest_locale_tests
ctest --test-dir build -R '^guest_locale$' --output-on-failure
git checkout HEAD -- core/libs/prx/libc/src/LocaleSupport.cppResults
| Check | Result |
|---|---|
Merge into current main |
Clean |
git diff --check |
No output |
check_conventions.py |
All conventions checks passed. |
Build of the six locale test targets and libc |
Passed |
Build of libs |
Passed (1941/1941) |
| Six focused locale suites | 6/6 passed |
Negative control (guest_locale against main's LocaleSupport.cpp) |
Fails as expected |
ctest, merged tree
1/6 Test #525: guest_multibyte_locale ........... Passed 0.00 sec
2/6 Test #526: guest_wide_conversion ............ Passed 0.00 sec
3/6 Test #522: guest_wide_formatting ............ Passed 0.00 sec
4/6 Test #659: guest_locale ..................... Passed 0.00 sec
5/6 Test #523: guest_wide_strings ............... Passed 0.00 sec
6/6 Test #566: guest_wide_stream_scan ........... Passed 0.00 sec
100% tests passed out of 6
MarcZX10's blocking item on 6ad55bff26 asked for a rebase and a rerun of guest_locale on current main. This head is rebased, and the run above is on the merge with current main ba5b5330b3, so that item is resolved.
Negative control
I restored only core/libs/prx/libc/src/LocaleSupport.cpp from origin/main and kept the PR's GuestLocale.cpp. After a rebuild, guest_locale aborted with Subprocess aborted (the test's Guest locale check failed). With the merged file restored, it passes again.
ctest, negative control
1/1 Test #659: guest_locale .....................Subprocess aborted***Exception: 0.08 sec
0% tests passed, 1 tests failed out of 1
The following tests FAILED:
659 - guest_locale (Subprocess aborted)
Findings
Blocking
None.
Non-blocking
_Mbtowcxstill throws where the description says the console returns a code. SeeLocaleSupport.cpp:864-871. The description says the console'sindelegates to_Mbtowcxand maps its results topartialanderror. If so, the console's_Mbtowcxreturns -2 for incomplete input and -1 for invalid input instead of aborting. The PR now routes_Mbtowcxthrough the guest helper, but it still throws on both results. The new tests atGuestLocale.cpp:244-253lock in the throws for"\xe2", a four-byte lead and"\xc3\x28". The port'sincallsmbrtowc_nid_postfixdirectly, so the facet is unaffected, but a guest that calls_Mbtowcxon a split UTF-8 sequence will abort. The throws were already there before this PR, anddocs/dev/debt/libc.mddoes not list them. Suggested fix: if the PPSA03446 disassembly confirms the codes, return -2 and -1 as the console does and assert those values in the test. Otherwise, add a debt entry for the throwing paths. This can be a follow-up.lengthreturns a character count.WideCodecvtLengthreturns the number of wide characters, and the test expects 3 for six bytes of input (GuestLocale.cpp:93). The standarddo_lengthreturns the number of bytes consumed. Older Dinkumware libraries did count characters, so this may well match the console. The debt entry says "counts characters", but it does not say that this differs from the standard. One sentence there, stating that the console behaviour was confirmed, would keep someone from "fixing" it to return bytes later.- Depends on: #10165. The description calls this dependency textual only, and the PR merges and passes cleanly on current
mainwithout #10165. Listing it under Depends on may hold this PR back behind an open PR it does not need. Considernone, and keep the rebase note in the text.
Not verified
- Console behaviour: slot order, the
max_lengthvalue of 6,partialversusokwheninfills the output after converting some characters, and the character-countlength. I have no PPSA03446libc.prxdisassembly to compare against. - A Windows build or run (16-bit host
wchar_tpaths). I ran on Linux only. - An ASan build.
00xDisaster
force-pushed
the
feat/libc-codecvt-wide-vtable
branch
from
October 30, 2026 22:56
fdc62fb to
c5f4351
Compare
Contributor
Author
|
@XairRrR thanks for the approval. Head is rebased onto current (). The six locale suites still pass 6/6 with the bundled Khronos validation-layer run; the new debt sentence notes that no console disassembly was available for 's -1/-2 returns or the character-count point you raised. I also changed to because the PR merges and passes on current without #10165. The Windows run remains Xair's independent evidence plus maskym's earlier result. Could you re-check this fresh head? The diff is otherwise the same codecvt implementation and tests. |
MarcZX10
suggested changes
Oct 30, 2026
MarcZX10
left a comment
MarcZX10
left a comment
Contributor
There was a problem hiding this comment.
Reviewed — changes requested
The new length implementation and test pin a character-count result even though the PR records that the console behavior was not verified.
What I ran
Reviewed exact head c5f43513fbc745807c08b1a0d3513b476ab8c16e against PR base b44985a3ea80f1e9618f7d925292229bf8f051a7. The individual merge preview against current main fc7c353fbcbc974d6b8af027c2769f9f63357b07 was clean. The reported test base 7fcab08f71e463b1768730a626a0f5472d336708 is the merge base of this head and current main; a comparison across the 106 intervening commits found no changes to the locale source, test, debt, or multibyte helper files. Environment: Debian Linux 6.1.0-54-amd64, x86-64, Python 3.11.2; no compiler or project build was run.
python3 tools/pr_precheck.py --repo boykopovar/AnyPS5 --pr 11172
gh pr checks 11172 --repo boykopovar/AnyPS5
python3 tools/pr_stack_check.py 11172 --repo boykopovar/AnyPS5
git merge-base 7fcab08f71e463b1768730a626a0f5472d336708 c5f43513fbc745807c08b1a0d3513b476ab8c16e
git fetch --no-tags https://github.com/boykopovar/AnyPS5.git refs/heads/main
gh api repos/boykopovar/AnyPS5/compare/7fcab08f71e463b1768730a626a0f5472d336708...fc7c353fbcbc974d6b8af027c2769f9f63357b07 --jq '[.files[].filename] | map(select(test("^(core/libs/prx/libc/src/LocaleSupport\\.cpp|core/libs/tests/GuestLocale\\.cpp|docs/dev/debt/libc\\.md|core/libs/prx/libc/src/Multibyte\\.cpp|core/libs/prx/libc/include/Api\\.hpp)$")))'
curl -fsSL https://raw.githubusercontent.com/cplusplus/draft/d4d6f38143c1b495aa33b2e5de0e0ec2daa173ca/source/text.tex -o /tmp/cppdraft-text.tex
grep -n -A28 -B1 '\\indexlibrarymember{codecvt}{do_length}' /tmp/cppdraft-text.texI also inspected the exact-head WideCodecvtLength, its test assertions, the updated debt entry, the precheck's convention warning, and the author-reported test scope.
Results
| Check | Result |
|---|---|
| Diff and conventions | Diff check passed. Conventions reported 0 errors and one warning: the new debt entry is 1,072 characters and should state the gap first. |
| Current-main integration | Clean individual merge preview. The reported test base is the current merge base, and the changed locale/test/debt paths have no later main changes. |
| Hosted CI | Eligibility, conventions, labels, library-portability, report, Windows, Ubuntu relinker, Ubuntu register-context, and ASan passed. macOS relinker, macOS register-context, and write-watch jobs were pending; Linux product and specialized Linux checks were skipped. |
| Author-reported tests | The PR reports six focused locale suites passing on code commit 230cc7a59d44c2e6e9001fbae32036d466578216, plus an ASan guest_locale run. I did not independently run them. The current head adds a debt-file update after that code commit. |
length behavior |
The test expects length(..., max=2) to return 2 for the six-byte input Aé€, and max=4 to return 3. The pinned C++ draft defines the result as the number of external characters consumed and specifies the state effect as if do_in were called. The PR's debt entry says the console length behavior was not disassembled. |
| Local build or tests | Not run. |
Unedited output from relevant checks
Precheck convention output:
docs/dev/debt/libc.md:238: warning: [debt-length] Technical debt entry over 1000 characters: state the gap first (what differs from the console, what is unverified, what throws) and keep only the behaviour it needs: 1072 characters
1 findings, 0 errors; GitHub shows at most 10 error annotations, the job summary lists all
Precheck aggregate result:
Conclusion: checks failed
The precheck's ABI and test-quality helpers were unavailable in its isolated snapshot (tools/pr_abi_audit.py and tools/pr_test_quality.py were not present there); those helper failures do not establish a PR defect. The source and test were inspected directly.
Stack preview:
# PR stack check — fresh
Target: https://github.com/boykopovar/AnyPS5/pull/11172
Target branch tip: `fc7c353fbcbc974d6b8af027c2769f9f63357b07`
All heads unchanged: **True**
Dependency order: #11172
## Individual merge previews
- PR #11172: **clean** against current base; head `c5f43513fbc745807c08b1a0d3513b476ab8c16e`; changed paths: none reported
## Dependency relationships
No declared PR dependencies were found in the target PR chain.
Hosted checks:
linux skipping 0 https://github.com/boykopovar/AnyPS5/actions/runs/41141416084/job/124356578490
relinker-asan pass 2m31s https://github.com/boykopovar/AnyPS5/actions/runs/41141416084/job/124356577141
register-context (macos-15-intel, Debug) pending 0 https://github.com/boykopovar/AnyPS5/actions/runs/41141416084/job/124356577079
relinker (macos-latest, clang++, x86_64) pending 0 https://github.com/boykopovar/AnyPS5/actions/runs/41141416084/job/124356577358
register-context (ubuntu-latest, Debug) pass 33s https://github.com/boykopovar/AnyPS5/actions/runs/41141416084/job/124356577183
register-context (ubuntu-latest, Release) pass 29s https://github.com/boykopovar/AnyPS5/actions/runs/41141416084/job/124356577062
relinker (windows-latest, g++) pass 6m38s https://github.com/boykopovar/AnyPS5/actions/runs/41141416084/job/124356577186
labels pass 8s https://github.com/boykopovar/AnyPS5/actions/runs/41141412689/job/124358585825
library-portability pass 1m45s https://github.com/boykopovar/AnyPS5/actions/runs/41141416046/job/124356311224
conventions pass 19s https://github.com/boykopovar/AnyPS5/actions/runs/41141412567/job/124356304483
relinker (ubuntu-latest, g++) pass 4m15s https://github.com/boykopovar/AnyPS5/actions/runs/41141416084/job/124356577235
relinker (macos-latest, clang++) pending 0 https://github.com/boykopovar/AnyPS5/actions/runs/41141416084/job/124356577245
register-context (macos-15-intel, Release) pending 0 https://github.com/boykopovar/AnyPS5/actions/runs/41141416084/job/124356577203
write-watch (macos-15-intel) pending 0 https://github.com/boykopovar/AnyPS5/actions/runs/41141416084/job/124356577152
write-watch (macos-15) pending 0 https://github.com/boykopovar/AnyPS5/actions/runs/41141416084/job/124356577069
eligibility pass 10s https://github.com/boykopovar/AnyPS5/actions/runs/41141416084/job/124356311738
report pass 1m0s https://github.com/boykopovar/AnyPS5/actions/runs/41141415992/job/124356313581
windows pass 7m34s https://github.com/boykopovar/AnyPS5/actions/runs/41141416084/job/124356577045
Findings
Blocking
WideCodecvtLength returns the number of decoded wide characters and leaves the caller's state unchanged by using a local copy. The new test locks in that behavior. The author’s latest comment acknowledges that the character-count behavior is unverified on the console, while the exact-head implementation and tests still assert it. This remains an unmeasured target-specific assertion, and it differs from the standard codecvt::do_length contract. Please either provide pinned binary/console evidence for the target's return value and state behavior and test those results, or leave this slot throwing and record the unresolved behavior until it is verified, as the repository requires for unknown behavior.
Non-blocking
The debt entry exceeds the convention's 1,000-character guidance and the checker asks that it lead with the unresolved gap. This is a warning, not a check failure.
Sources
- Repository rule for unknown behavior at the reviewed head
- Wide
lengthimplementation at the reviewed head - Assertions that pin the returned character count
- Debt entry documenting the unverified console behavior
- C++ draft
codecvt::do_length, pinned to commitd4d6f38143c1b495aa33b2e5de0e0ec2daa173ca
Not verified
I did not build or run C++ tests locally or test on a PS5. The six focused suite results and ASan run are author-reported; the current hosted macOS jobs were still pending when checked.
To get this to a good state
Please verify length against the pinned target binary or console, or keep this slot unsupported until its behavior is known. I will rerun the relevant checks against the new head.
The wide codecvt data object added by boykopovar#10180 was still thirteen zeroed slots. Fill slots 2 through 12 with the facet methods and the seven conversion virtuals: always_noconv is false, max_length is 6, encoding is 0, in and out convert UTF-8 of at most three bytes, unshift clears the state, and length counts characters without changing the caller's state. The console's wide codecvt virtuals delegate to _Mbtowcx and _Wctombx, so route those exports through the guest UTF-8 helpers instead of the host C locale, and reserve the 16-byte guest conversion state that _Getpmbstate and _Getpwcstate return. Cover the exported object, the conversion results, a partial output buffer, an invalid four-byte lead, the length limit and unshift.
00xDisaster
force-pushed
the
feat/libc-codecvt-wide-vtable
branch
from
October 31, 2026 20:36
c5f4351 to
597258f
Compare
Contributor
Author
|
@MarcZX10 I took the second option: the
The rest of the vtable is unchanged:
One CI note: both macOS |
Collaborator
|
Reviewed at Please remove the new |
# Conflicts: # docs/dev/debt/libc.md
Contributor
✅ AMD GPU test: passedPR head
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (13, not counted against this PR)Validation IDs already present on main (5 IDs in 711 tests)Skipped (57)ReproduceAutomated build and test on a contributor's AMD machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
# Conflicts: # core/libs/prx/libc/src/LocaleSupport.cpp
github-actions
Bot
added
needs rebase
Contributor
|
Retested head Host: Windows 11 26300, Intel i7-12700K, WinLibs GCC 15.2.0 (x86_64-ucrt-posix-seh, r6), Release,
AI-assisted: yes (Claude Code), the runs and the numbers above are from this machine. |
XairRrR
approved these changes
Nov 2, 2026
XairRrR
left a comment
XairRrR
left a comment
Contributor
There was a problem hiding this comment.
Re-checked — approved
Since fdc62fb104, the length slot throws instead of returning an unverified character count, the debt entry records that gap, and the checklist now reads Depends on: none. The merged tree builds, the six locale suites pass, and guest_locale still fails against main's LocaleSupport.cpp.
What changed since my last review
597258ffad:WideCodecvtLengthnow only throws (codecvt<wchar_t,char>::length: console behavior unverified). The test no longer expects2/3forAé€; it now requires the slot to throw.6d06d26606: the debt entry leads with thelengthgap, names PPSA03446'slibc.prxas the source of the layout and of the_Mbtowcx/_Wctombxdelegation, and notes that offset-to-top and typeinfo stay zero.- Two merge commits from
mainresolve conflicts indocs/dev/debt/libc.mdandLocaleSupport.cpp. Thein/out/unshiftcode and its tests are unchanged. - Description:
Depends on: none(previously #10165), and a new Windows rerun by maskym ond7562dcc1d(guest_localepassed 3 runs; author-reported). - Discussion: MarcZX10 asked for
lengthto throw until it is measured, which is now done. oneandonlydean then asked for theRequireExceptiononlengthto be removed from the test; that has not been done (see below).
What I ran
Head d7562dcc1d873ad19809c9b31d350de94db517c9 merged with --no-ff into main 096c45570f516de76a43bbe1b97781381c84f55f (merged tree 4d8e99a644e1bb9ff664b93f011e943a152cd705). Debian 13 VM, GCC 14.2.0, Release, Ninja, BUILD_TESTING=ON.
git diff --check 096c45570f516de76a43bbe1b97781381c84f55f...d7562dcc1d873ad19809c9b31d350de94db517c9
python3 tools/check_conventions.py --base 096c45570f516de76a43bbe1b97781381c84f55f
ctest --test-dir build -R '^(guest_locale|guest_multibyte_locale|guest_wide_formatting|guest_wide_strings|guest_wide_conversion|guest_wide_stream_scan)$' --output-on-failure
# negative control
git show 096c45570f516de76a43bbe1b97781381c84f55f:core/libs/prx/libc/src/LocaleSupport.cpp > core/libs/prx/libc/src/LocaleSupport.cpp
ninja -C build guest_locale_tests
ctest --test-dir build -R '^guest_locale$' --output-on-failure
git show 4d8e99a644e1bb9ff664b93f011e943a152cd705:core/libs/prx/libc/src/LocaleSupport.cpp > core/libs/prx/libc/src/LocaleSupport.cpp
ninja -C build guest_locale_tests
ctest --test-dir build -R '^guest_locale$'
git status --porcelain| Check | Result |
|---|---|
git diff --check |
No output |
check_conventions.py |
All conventions checks passed. |
Build and libs (script) |
Passed |
| Selected tests (script, 3496) | No new failures and no timeouts. The 9 known failures also fail on main (none are locale tests) |
| Six locale suites | 6/6 passed |
| Negative control | guest_locale aborts with main's LocaleSupport.cpp and passes again with the merged file restored. The worktree is clean afterwards |
ctest, negative control
0% tests passed, 1 tests failed out of 1
The following tests FAILED:
797 - guest_locale (Subprocess aborted)
Previous findings
lengthreturns a character count: Resolved.WideCodecvtLengthnow throws. The debt entry states that the return value and state effect are unverified and that the standarddo_lengthconsumes input bytes. This also answers MarcZX10's blocking item onc5f43513fb.- Depends on: #10165: Resolved. The checklist now reads
none, and the description keeps the textual rebase note. The PR merges and passes on currentmainwithout #10165. _Mbtowcxthrows on incomplete or invalid input: Still open, non-blocking.LocaleSupport.cpp:932-937still throws on the -1 and -2 results.GuestLocale.cpp:243-252still asserts those throws. The debt entry saysin/outdelegate to_Mbtowcx/_Wctombxon the console, which suggests the console returns codes there, but it does not list the throwing paths. Suggested fix: add the throws to the debt entry, or return the console's codes once they are confirmed. This can be a follow-up.
New findings
Blocking
None.
Non-blocking
- The test requires the unimplemented
lengthslot to throw. AtGuestLocale.cpp:92,RequireException([&] { vtable->length(...); })is still present at this head, although oneandonlydean asked on597258ffadfor it to be removed. Whoever later implementslengthwould have to delete it. I agree with their request: drop that line and keep the positivein/out/unshiftchecks and the state checks. This is not blocking because no written rule in CONTRIBUTING.md or AGENTS.md forbids it.
Not verified
- Console behaviour of the slot order,
max_length6, thepartial/errormapping andlength. I have no PPSA03446libc.prxdisassembly. - Windows (16-bit
wchar_t) and ASan runs. Both are author- or contributor-reported only (maskym's Windows rerun ond7562dcc1d; the author's ASan run on an earlier head). - Validation-layer comparison: not applicable, because the change touches no AGC driver, shader recompiler or VideoOut file.
12 tasks done
Contributor
✅ AMD GPU test: passedPR head
Unstable on main (2, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (13, not counted against this PR)Passed on retry (1, failed in the parallel run, passed when re-run with a 360 s timeout; not counted against this PR)Validated but not compared (64, not in the main baseline's validation pass)Validation IDs already present on main (8 IDs in 800 tests)Skipped (62)ReproduceAutomated build and test on a contributor's AMD machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Wire the exported
codecvt<wchar_t,char,_Mbstatet>vtable. #10180 added the thirteen-slot_ZTVSt7codecvtIwc9_MbstatetE_nid_postfixdata object (NIDH-TDszhsYuY,st_size104) with every slot zero, so a title that resolved the object and calledin,outorunshiftfaulted. Slots 2 through 11 now hold the same facet methods and conversion virtuals that the console uses; slot 12 (length) throws.The slot order and the conversion behaviour come from the
libc.prxbundled with PPSA03446; the size and facet layout were already recorded from PPSA26344 andlibSceLibcInternal.sprx. Slots 2 to 5 are the facet destructor, deleting destructor, retain and release; slot 6 returns false foralways_noconv; slot 7 returns 6 formax_length; slot 8 returns 0 forencoding; slots 9 to 12 arein,out,unshiftandlength. The console'sinandoutdelegate to its_Mbtowcxand_Wctombx, so this port does the same through the guest UTF-8 helpers:indecodes one- to three-byte UTF-8,outencodes one to three bytes, four-byte leads are invalid, a partial output buffer returnspartial, an invalid sequence returnserror, andunshiftclears the state.The
lengthslot throws instead of returning a character count. Standarddo_lengthconsumes input bytes rather than wide characters, and no disassembly or console run established this module's return value or its state effect, so the slot stays unsupported until that evidence exists. The debt entry records the gap._Mbtowcxand_Wctombxpreviously used the hostmbrtowc/wcrtomb, which depends on the host C locale and cannot produce the guest's three-byte UTF-8. They now use the guest helpers, and_Getpmbstateand_Getpwcstatereserve the 16-byte conversion state those helpers read and write. Slots 0 and 1 remain zero, as they were; the debt entry records that and the console's relocated offset-to-top and typeinfo entries.This does not change the narrow
codecvt<char>object in #10165, thenum_putwork in #11054, thecollate<char>object in #11166, or thefgetwcsurrogate handling in #10929, and no open PR implements these wide conversion virtuals. The only overlap is textual: #10165 resizes and moves the adjacent narrow codecvt declaration, so whichever lands second needs a rebase; this PR is listed after #10165 so the older change can land first.Tested
Linux CachyOS 7.3.0-4-cachyos, GCC 16.2.1. Base
upstream/main0836dfdf4fecb1e0326100c490476e15d3a74da5; head597258ffad.cmake --build build --target guest_locale_tests guest_multibyte_locale_tests guest_wide_conversion_tests guest_wide_formatting_tests guest_wide_string_tests guest_wide_stream_scan_tests --parallel: passed.ctest --test-dir build -R '^(guest_locale|guest_multibyte_locale|guest_wide_formatting|guest_wide_strings|guest_wide_conversion|guest_wide_stream_scan)$' --output-on-failure: 6/6 passed after the rebase.guest_localecalls the exported vtable'sin,outandunshift, checks ASCII, two- and three-byte UTF-8, an incomplete lead completed by the next call, a partial output buffer, an invalid four-byte lead, the facet retain/release slots, and that slots 0 and 1 stay zero. It callslengthand asserts that it throws.tools/check_conventions.py --base upstream/main --head HEADandgit diff --check upstream/main..HEAD: clean.write-watchjobs fail before building this PR:core/libs/tests/write_watch/CMakeLists.txtregisterswrite_watch_execute-faultandwrite_watch_read-faulttwice. The same duplicate loop is present onupstream/main; this PR does not touch that file, so it is a base-branch CI issue rather than an effect of this change.guest_localepassed in a joint 160/160 run of the libc, CesCs, Font, FreeType, LibreSsl3 and NpUniversalDataSystem suites. That run predates the refresh; the refreshed head is tested on Linux above. Not run on a PS5.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)